Skip to content

Make it possible to choose between multiple planetary HiPS per planet#4459

Merged
10110111 merged 10 commits intomasterfrom
planetary-hips-gui
Aug 23, 2025
Merged

Make it possible to choose between multiple planetary HiPS per planet#4459
10110111 merged 10 commits intomasterfrom
planetary-hips-gui

Conversation

@10110111
Copy link
Copy Markdown
Contributor

@10110111 10110111 commented Aug 17, 2025

Description

Currently we can only have one HiPS pack per planet (albedo, normals, horizons). This PR enables multiple HiPS per planet to choose from.

The surveys are arranged into groups, with the groups defined either by group property of a set of surveys, or by the hipslist's URL.

To test this, you can use my new Moon survey pack: http://86.106.181.97/hips/hipslist. Additional set of surveys can be obtained from this URL (with a nuance that their coordinate system is shifted by 180° of longitude with respect to ours, which should be taken care of in a later change). And there's also the default Stellarium surveys that are normally removed if you set custom ones in the config file: one, two.

If you are going to merge this, please keep the commits separate.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change requires a documentation update
  • Housekeeping

How Has This Been Tested?

Test Configuration:

  • Operating system: Ubuntu 25.04
  • Graphics Card: AMD Ryzen AI 9 HX 370 w/ Radeon 890M

Checklist:

  • My code follows the code style of this project.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (header file)
  • I have updated the respective chapter in the Stellarium User Guide
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@github-actions
Copy link
Copy Markdown

Great PR! Please pay attention to the following items before merging:

Files matching src/**/*.cpp:

  • Are possibly unused includes removed?

This is an automatically generated QA checklist based on modified files.

@gzotti
Copy link
Copy Markdown
Member

gzotti commented Aug 17, 2025

Nice addition! I like the various moon maps. Any clue why they are reversed? (Or why ours are reversed if the other ones fulfill some standard?)
I have not found the logic, but sometimes (no, actually often) when activating nomenclature (crater symbols&names) it seems one survey is stuck and does not disable. Then after clicking around some more, disabling nomenclature, it suddenly works again. Not sure if and how nomenclature could have anything to do with that, though. Maybe switch off nomenclature (how/why?) and move time by a few minutes also helped?

Also, frequently a message pops up "This group of surveys doesn't have an albedo map. Can't display it". I then have these trouble to disable displaying the survey.

@10110111
Copy link
Copy Markdown
Contributor Author

10110111 commented Aug 18, 2025

I have not found the logic, but sometimes (no, actually often) when activating nomenclature (crater symbols&names) it seems one survey is stuck and does not disable.

I don't reproduce this. Could you make a screencast or something to show this?

Any clue why they are reversed? (Or why ours are reversed if the other ones fulfill some standard?)

I guess theirs should be right, after all they are more actively interacting with the authors of the HiPS standard. As to why ours is different, I suppose it's because Guillaume didn't know better when he was implementing it, and now it's a matter of backwards compatibility. Now I also wonder if I should follow the reversal in my Moon survey too, before it's too late.

Also, frequently a message pops up "This group of surveys doesn't have an albedo map. Can't display it". I then have these trouble to disable displaying the survey.

OK, the failure to disable the survey after the message is a bug, I'll fix it soon.

As for the frequent popup, it's because you're supposed to disable the group if you don't want any survey to be shown, or just enable another survey from the group if you want to switch to it (the previously selected one will be unchecked automatically). Maybe there's a way to make UX better, any ideas?

@Atque
Copy link
Copy Markdown
Contributor

Atque commented Aug 18, 2025

How does one add your Moon survey? It doesn't appear in the list.

@10110111
Copy link
Copy Markdown
Contributor Author

How does one add your Moon survey?

SUG chapter 10.2.

It doesn't appear in the list.

It should appear by the group name: Moon horizons + normals + albedo for Stellarium.

@alex-w alex-w added the enhancement Improve existing functionality label Aug 18, 2025
@alex-w alex-w added this to the 25.3 milestone Aug 18, 2025
@github-actions
Copy link
Copy Markdown

Hello @10110111!

Thank you for the suggested improvement.

10110111 and others added 3 commits August 19, 2025 22:06
The planetary surveys from alasky.cds.unistra.fr appear to have a
reference frame that's rotated by 180° in longitude relative to that
of Stellarium's planetary surveys. These surveys were created by
Aladin/HipsGen, which I assume to be more or less a reference
implementation.

Since we can't easily change Stellarium's frame without backwards
compatibility issues, and the "type" field is a custom field not used
outside of Stellarium, we can use it as an indicator whether we need to
fixup the coordinates of a survey.
@10110111
Copy link
Copy Markdown
Contributor Author

After a few more commits it's even more important to keep them separate when merging. But at least now I've addressed all the comments here as well as a few issues I've found. Are there any objections to merging this?

@10110111 10110111 merged commit 56b051a into master Aug 23, 2025
31 of 32 checks passed
@10110111 10110111 deleted the planetary-hips-gui branch August 23, 2025 12:53
@10110111
Copy link
Copy Markdown
Contributor Author

10110111 commented Aug 24, 2025

Any clue why they are reversed? (Or why ours are reversed if the other ones fulfill some standard?)

Just stumbled upon this code in Planet::drawSurvey:

// Apply a rotation otherwise the hips surveys don't get rendered at the
// proper position.  Not sure why...
painter->getProjector()->getModelViewTransform()->combine(Mat4d::zrotation(M_PI * 0.5));

...

// Undo the rotation we applied for the survey fix.
v = Mat4d::zrotation(M_PI * 0.5) * v;

And this comes from the very beginning, #110, namely this commit. I'm still not sure what exactly would go wrong if this kludge weren't included. Maybe Guillaume thought that his HiPS were correctly generated while they weren't, and introduced this workaround into the C++ code, breaking the actual HipsGen-generated surveys.

In any case, this is working now, but the code contains two rotations: one in Planet::drawSurvey, and another in StelHips.cpp!shiftPix180deg, which may be worth trying to simplify in the future.

@alex-w alex-w added the state: published The fix has been published for testing in weekly binary package label Sep 17, 2025
@github-actions
Copy link
Copy Markdown

Hello @10110111!

Please check the fresh version (development snapshot) of Stellarium:
https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot

@alex-w alex-w removed the state: published The fix has been published for testing in weekly binary package label Sep 29, 2025
@github-actions
Copy link
Copy Markdown

Hello @10110111!

Please check the latest stable version of Stellarium:
https://github.com/Stellarium/stellarium/releases/latest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improve existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants